Skip to content

Conversation

floryst
Copy link
Contributor

@floryst floryst commented Jul 31, 2025

Context

The glyphmapper does not handle large coordinates, resulting in jittering

Results

Large coordinates no longer jitter when using the glyph3dmapper

Changes

  • Glyph3DMapper now applies a shift+scale when rendering large coordinates

PR and Code Checklist

  • semantic-release commit messages
  • Run npm run reformat to have correctly formatted code

Testing

  • This change adds or fixes unit tests
  • Tested environment:
    • vtk.js:
    • OS:
    • Browser:

@floryst floryst requested review from sankhesh and Copilot and removed request for Copilot July 31, 2025 17:34
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes jittering issues in the Glyph3DMapper when rendering objects with large coordinates by implementing a shift+scale transformation approach.

  • Adds coordinate shift and scale computation to handle large coordinate values
  • Applies transformations to both matrix buffers and primitive coordinate buffers
  • Ensures the Glyph3DMapper's shift+scale takes precedence over the base mapper's transformation

@floryst floryst force-pushed the glyphmapper-shift-scale branch from 378955e to e11d41b Compare July 31, 2025 17:41
Copy link
Member

@finetjul finetjul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (from a syntax point of view)

@daker
Copy link
Collaborator

daker commented Aug 1, 2025

@floryst some tests are failing

'Test Suite: Test getPixelWorldHeightAtCoord'
2025-07-31T17:47:44.3348969Z Chrome 138.0.0.0 (Linux x86_64) INFO: '	[PASS] Image match resolution'
2025-07-31T17:47:44.3350652Z Chrome 138.0.0.0 (Linux x86_64) INFO: '	[FAIL] [Widgets/Core/WidgetManager/test/testNoScaleInPixelsWithPerspective] Matching image - delta 97.56% (count: 87808)'
2025-07-31T17:47:44.3753081Z Chrome 138.0.0.0 (Linux x86_64) INFO: '	[PASS] Image match resolution'
2025-07-31T17:47:44.3761310Z Chrome 138.0.0.0 (Linux x86_64) INFO: '	[FAIL] [Widgets/Core/WidgetManager/test/testNoScaleInPixelsWithParallel] Matching image - delta 88.08% (count: 79271)'
2025-07-31T17:47:44.4061230Z Chrome 138.0.0.0 (Linux x86_64) INFO: '	[PASS] Image match resolution'
2025-07-31T17:47:44.4069023Z Chrome 138.0.0.0 (Linux x86_64) INFO: '	[FAIL] [Widgets/Core/WidgetManager/test/testScaleInPixelsWithPerspective] Matching image - delta 95.34% (count: 85807)'
2025-07-31T17:47:44.4265524Z Chrome 138.0.0.0 (Linux x86_64) INFO: '	[PASS] Image match resolution'
2025-07-31T17:47:44.4273077Z Chrome 138.0.0.0 (Linux x86_64) INFO: '	[FAIL] [Widgets/Core/WidgetManager/test/scaleInPixelsWithParallel] Matching image - delta 64.24% (count: 57813)'

@floryst
Copy link
Contributor Author

floryst commented Aug 4, 2025

Yeah I'm looking into why the tests are failing.

@floryst floryst force-pushed the glyphmapper-shift-scale branch from e11d41b to 2bf3702 Compare August 12, 2025 23:54
@floryst
Copy link
Contributor Author

floryst commented Aug 13, 2025

Tests have passed. This is good for review @sankhesh

Copy link
Collaborator

@sankhesh sankhesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sankhesh sankhesh added this pull request to the merge queue Aug 13, 2025
Merged via the queue into master with commit cc07c84 Aug 13, 2025
2 checks passed
Copy link

🎉 This PR is included in version 34.8.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Automated label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants